[diffs] Tests Audit#801
Conversation
Consolidate the JSDOM bootstrap (installDom, MockResizeObserver, MockPointerEvent, rAF shim) and common helpers (createRoot, wait, dispatchScroll, makeFile, makeFileItem, renderItems) that were copy-pasted across 12 test files into test/domHarness.ts. The shared harness always installs the same superset of globals, since per-file subsets had drifted apart and caused harness bugs. Pure refactor: no test logic or assertions changed; removes ~1,400 duplicated lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add behavioral projection helpers to testUtils (projectColumn, projectRenderResult, rowDigests, hunkDigest, patchDigest, annotationProjection, collectRowSourceMismatches, countDeclaredRows) so tests can assert just the rows/indices/types/text they own instead of snapshotting entire render results. Remove findBufferElements: its data-virtualizer-buffer attribute only ever exists on live DOM nodes, never in renderer HAST, so every assertion built on it was vacuous. Rewrite DiffHunksRendererVirtualization.test.ts: - 1.1/1.2 assert the renderer's bufferBefore/bufferAfter passthrough instead of the impossible buffer elements, and document the row-count derivation (514 hunk rows + 3 auto-expanded single-line gaps = 517). - 3.5 now proves the a9ff17b7 regression guard with a differential oracle (windowed render === rows 30..79 of the full render) plus an explicit containment check on the expanded indices 57-76, instead of a 2.8k-line snapshot. - 6.1/6.2 finally do what their names promise: rendered window rows are compared to the full render slice and their text to the exact source lines on the correct side. - Delete test 2.1 (the 517 count was pinned three times) and 7.2 (after removing its redundant full render it duplicated 7.1 verbatim); simplify 4.4's dead else-branch and 7.1's decorative toBeDefined. - Correct the stale fixture geometry comments (the header undercounted rendered rows and misnumbered hunk ranges) and document that startingLine/totalLines index renderable rows, not virtual indices. Deletes the 2.1MB snapshot file; the remaining assertions are exact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DiffHunksRender.test.ts: the buffer-prepending tests now assert the named behavior directly — buffer rows appear in the correct column with sizes equal to each change block's add/delete surplus, derived from the parsed hunkContent — instead of burying it in 2,300-line full-HAST snapshots. All seven whole-result snapshots become compact rowDigests projections (or are dropped where countInlineDiffSpans already pins the behavior), and the five 'parsed diff' snapshots become verifyHunkLineValues invariant checks. annotations.test.ts: the three full-AST snapshots (151k lines frozen per-token theme styles) become annotationProjection snapshots — the ordered (line, annotation, slots) placement record the tests are actually about; the precise loop assertions above them are unchanged. patchFileRender.test.ts: the known-tricky-patch regression pin is now a patchDigest + rendered-row projection instead of two whole-object snapshots, with hunk invariants checked explicitly. Also fixes an edge in verifyHunkLineValues surfaced by the new invariant checks: a diff whose addition side is empty (file deleted to nothing) has additionStart/additionCount 0, and the unclamped lastHunkEnd of -1 invented a phantom trailing context line. Snapshot churn: 264k lines of full-HAST output replaced by 265 lines of reviewable projections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parseMergeConflictDiffFromFile: the three whole-result snapshots of the 20,000-line conflict fixture (12.5MB, 607k lines — unreviewable and update-blessed by definition) become explicit assertions at each maxContextLines width: hunk invariants, diff sides exactly equal to the conflict-free current/incoming texts, no markers on either side, 44 actions (one per fixture conflict), monotonic hunk-row growth with wider context, plus a compact per-width geometry digest. The test name no longer promises "timing" it never measured (the benchmark script owns timing). parsePatchFiles: the 94k-line whole-result snapshot of the 400KB diff.patch becomes a one-line-per-hunk patchDigest snapshot; the small targeted snapshots (final blank line, malformed patch) stay. Invariant checks now assert .errors toEqual([]) so failures print the actual errors instead of valid === false. parseDiffFromFile: full-result snapshot becomes a hunkDigest snapshot. hunkDigest now renders each hunk as a single line, keeping the 400KB patch's digest reviewable (~1.5k lines instead of 7.4k). test/__snapshots__ total: 966k lines / 20MB before this series, now 4.5k lines / 156K. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FileRenderer.test.ts keeps its full highlighted-AST snapshot and is now documented as the suite's single full-fidelity token/style canary: theme or tokenizer changes should churn exactly this one snapshot, which at 1.6k lines is small enough to review line by line. FileRenderer.ast.test.ts: the css check accepted any string while the non-worker path hardcodes '' — assert that contract explicitly; the preNode test asserted no actual property — it now checks data-file, data-overflow, tabIndex, and the gutter-width style derivation; the totalLines test no longer re-asserts file2 (covered by the structure test above it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cannot-fail assertions, each replaced with one that discriminates:
- CodeView.collapsed: top <= bottom on window specs is guaranteed by
createWindowFromScrollPosition's clamping; now asserts the clamped
window reaches the list tail and scrollTop stays within the derived
collapsed content height. Also drops the sticky-specs test whose
expected value was read from the instance under test.
- CodeView.pointerEvents: the "cancels pending restore work" half
could never fail (post-cleanUp restores are no-ops); renamed to what
it verifies and dropped the dead post-wait re-assertions plus the
incidental intermediate-state pins.
- hydration: not.toBe('0px') passed with no placeholder at all; now
requires the placeholder and asserts its exact height derived from
DEFAULT_VIRTUAL_FILE_METRICS and parsed row counts.
- InjectedRowHooks: isHastElement on a helper that only returns hast
elements replaced with exact action-button table (action ids,
conflict indices); rowCount now derived from parser output instead
of the rendered result itself.
- CodeView.rangeScroll: line/range equivalence now anchors on the
exact derived center position so both paths silently no-oping at 0
can no longer pass.
Magic numbers replaced with derivations:
- iterateOverDiff: 517/490 become countDeclaredRows(diff, style)
(hunk-declared rows + auto-expanded gaps at the collapsed-context
threshold); fixes the split test collecting unifiedLineIndex (a
copy-paste bug nothing caught because it was never asserted) and
adds a metadata-derived split-pairing assertion; deletes the shallow
big-fixture expansion check superseded by the synthetic-fixture
tests.
- computeEstimatedDiffHeights: every raw expected number (36/64/.../
1454) rewritten as arithmetic over the named metrics and parsed
fixture geometry, in the style of virtualFileMetricsPadding.
- CodeView.scrollAnchoring: verbatim 2,000,000/12,000,000px pins
become named mirrors of the source rebase constants with
relationship assertions (delta advancement, threshold-derived
bounds).
- virtualizedFileDiffEstimatedHeights: inline 5_000 named as
LAYOUT_CHECKPOINT_INTERVAL pointing at the source constant.
Truth-in-naming and accuracy:
- CodeView.updateItemId now asserts the rename's positive effects
(same element/instance under the new id only, retargeted pending
scroll actually scrolls).
- FileDiff.partialRender and sharedHighlighter tests renamed to what
they verify; advancedStickySpecs window-helper comments un-inverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the shared harness/projection conventions, the single-canary snapshot policy, and the coverage gaps confirmed by the test audit (worker pipeline, SSR hydration round trip, ScrollSyncManager, real Virtualizer, drag selection, shiki-stream, and friends) so the next person touching those areas knows the tests are missing rather than assuming they exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9118abe078
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Used a ton of tokens to have Fable go over all our diffs tests and see if it could clean things up.